Skip to content

feat: remove MaxMindBinary Provider#386

Merged
norkunas merged 1 commit intogeocoder-php:masterfrom
Chris53897:feature/remove-maxmind-binary-provider
Dec 22, 2025
Merged

feat: remove MaxMindBinary Provider#386
norkunas merged 1 commit intogeocoder-php:masterfrom
Chris53897:feature/remove-maxmind-binary-provider

Conversation

@Chris53897
Copy link
Contributor

@Chris53897 Chris53897 commented Dec 22, 2025 •
edited by coderabbitai bot
Loading

https://packagist.org/packages/geocoder-php/maxmind-binary-provider#4.4.0 is based on the abondend https://packagist.org/packages/geoip/geoip

Maybe we can rid of this, as a new major release is needed anyway.

Summary by CodeRabbit

  • Chores
    • Removed MaxMind Binary geocoder provider and geocoder-php/maxmind-binary-provider dependency
    • Removed GeoIP helper files from test bootstrap
  • Documentation
    • Removed MaxMind provider from services guide
  • Tests
    • Removed MaxMind and GeoIP provider tests and configuration

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025 •
edited
Loading

Walkthrough

This PR removes support for the MaxMind Binary provider from BazingaGeocoderBundle by deleting the factory class, eliminating its dev dependency from composer.json, removing test cases and configuration files, and updating documentation.

Changes

Cohort / File(s) Summary
Dependency Management
composer.json		 Removed dev dependency geocoder-php/maxmind-binary-provider (^4.4)
Documentation
doc/services.md		 Removed MaxMindBinaryFactory entry and its configuration options (dat_file, open_flag) from the Provider Factories table
Provider Factory Implementation
src/ProviderFactory/MaxMindBinaryFactory.php		 Deleted factory class that extends AbstractFactory, including static $dependencies property, getProvider() method, and configureOptionResolver() method
Test Cases & Configuration
tests/Functional/ProviderFactoryTest.php, tests/Functional/config/provider/maxmindbinary.yml		 Removed test imports (Geoip, MaxMindBinary), eliminated corresponding provider test yields, and deleted YAML configuration block for maxmindbinary provider with dat_file reference
Test Bootstrap
tests/bootstrap.php		 Removed two require_once statements for GeoIP helper files (geoip.inc and geoipcity.inc)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify that no other code paths depend on MaxMindBinaryFactory or the removed provider configuration
  • Confirm whether Geoip provider removal in tests is intentional and fully coordinated with this change
  • Check that all related configuration and test utilities have been cleaned up consistently

Possibly related PRs

Suggested reviewers

  • norkunas

Poem

🐰 A MaxMind binary hops away,
Its factory fades from yesterday,
Tests and configs bid adieu,
The bundle lighter—clean and new!
Removals done with care and grace,
A simpler, swifter codebase. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: remove MaxMindBinary Provider' accurately describes the main change - removal of the MaxMindBinary provider across all related files (factory, tests, config, dependencies).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ceaf27 and 38ff2fa.

📒 Files selected for processing (6)
  • composer.json
  • doc/services.md
  • src/ProviderFactory/MaxMindBinaryFactory.php
  • tests/Functional/ProviderFactoryTest.php
  • tests/Functional/config/provider/maxmindbinary.yml
  • tests/bootstrap.php
💤 Files with no reviewable changes (6)
  • composer.json
  • doc/services.md
  • src/ProviderFactory/MaxMindBinaryFactory.php
  • tests/Functional/ProviderFactoryTest.php
  • tests/Functional/config/provider/maxmindbinary.yml
  • tests/bootstrap.php

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

norkunas
norkunas reviewed Dec 22, 2025
View reviewed changes
Copy link
Member

@norkunas norkunas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we deprecate it in 5.x?

@Chris53897
Copy link
Contributor Author

Chris53897 commented Dec 22, 2025

We did not deprecate the other factories. #380
But we should add Changelog-Entries

@norkunas
Copy link
Member

norkunas commented Dec 22, 2025

Just a question - even if it's abandoned, maybe it still does work?

@Chris53897
Copy link
Contributor Author

Chris53897 commented Dec 22, 2025 •
edited
Loading

I do not know if this still works. Latest stable release is from 2016. Retired in 2022
https://blog.maxmind.com/2020/06/retirement-of-geoip-legacy-downloadable-databases-in-may-2022/

User can use an old version of this bundle and migrate to another provider.
I think users should migrate and this should not be supported anymore in this repo.
But this is just my personal view.

@norkunas
Copy link
Member

norkunas commented Dec 22, 2025

Ok, I agree

@norkunas norkunas merged commit 6bee0c0 into geocoder-php:master Dec 22, 2025
16 checks passed
@norkunas
Copy link
Member

norkunas commented Dec 22, 2025

Thank you @Chris53897

@Chris53897 Chris53897 deleted the feature/remove-maxmind-binary-provider branch December 22, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@norkunas norkunas norkunas approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Chris53897 @norkunas @Chris8934

Comments